avahi: Be robust to missing refs in peer summaries

In the OstreeRepoFinderAvahi implementation,
ostree_avahi_service_build_repo_finder_result() is where the DNS-SD
records are processed and turned into OstreeRepoFinderResult objects.
Each result object is supposed to have a hash table mapping refs to
checksums, so this is accomplished by first adding a placeholder (a ref
mapping to a NULL checksum) for each ref matched by the bloom filter,
and later filling in the checksums using the remote's summary file,
which happens in get_checksums(). The problem is that there's no
guarantee all the checksums will be resolved (non-NULL), so the
ostree_repo_finder_result_new() call then hits an assertion failure in
is_valid_collection_ref_map() leading to a crash (in the case that one
or more refs had NULL checksums).

There are at least two situations where the ref checksum might not be
found in the peer remote's summary file:
1) The bloom filter match was a false positive. This is going to happen
sometimes by design.
2) The peer remote's summary is out of sync with its DNS-SD records.
This shouldn't normally happen but it's still good to be robust to the
possibility; in Endless OS nothing guarantees the atomicity of updating
the summary and DNS-SD records.

This commit changes libostree to be robust to the possibility of refs
missing from the peer remote's summary, by removing any that still have
a NULL checksum associated with them after the summary has been fetched
and processed.

The other OstreeRepoFinder implementations don't have this issue because
they use summary files directly and therefore always have access to the
checksum.

Closes: #1717
Approved by: pwithnall

diff --git a/src/libostree/ostree-repo-finder-avahi.c b/src/libostree/ostree-repo-finder-avahi.c
index bc38376974e25070a9c8045e22dcd2064a869015..f8d1f878ef313f0e7184200228b82383ad5e6822 100644 (file)
--- a/src/libostree/ostree-repo-finder-avahi.c
+++ b/src/libostree/ostree-repo-finder-avahi.c
@@ -509,6 +509,16 @@ fill_refs_and_checksums_from_summary (GVariant    *summary,
   return TRUE;
 }
 
+static gboolean
+remove_null_checksum_refs_cb (gpointer key,
+                              gpointer value,
+                              gpointer user_data)
+{
+  const char *checksum = value;
+
+  return (checksum == NULL);
+}
+
 /* Given a summary file (@summary_bytes), extract the refs it lists, and use that
  * to fill in the checksums in the @supported_ref_to_checksum map. This includes
  * the main refs list in the summary, and the map of collection IDs to further
@@ -518,14 +528,13 @@ fill_refs_and_checksums_from_summary (GVariant    *summary,
  * set and %FALSE will be returned. If the intersection of the summary file refs
  * and the keys in @supported_ref_to_checksum is empty, an error is set. */
 static gboolean
-get_refs_and_checksums_from_summary (GBytes      *summary_bytes,
-                                     GHashTable  *supported_ref_to_checksum /* (element-type OstreeCollectionRef utf8) */,
-                                     GError     **error)
+get_refs_and_checksums_from_summary (GBytes       *summary_bytes,
+                                     GHashTable   *supported_ref_to_checksum /* (element-type OstreeCollectionRef utf8) */,
+                                     OstreeRemote *remote,
+                                     GError      **error)
 {
   g_autoptr(GVariant) summary = g_variant_ref_sink (g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, summary_bytes, FALSE));
-  GHashTableIter iter;
-  const OstreeCollectionRef *ref;
-  const gchar *checksum;
+  guint removed_refs;
 
   if (!g_variant_is_normal_form (summary))
     {
@@ -544,20 +553,20 @@ get_refs_and_checksums_from_summary (GBytes      *summary_bytes,
   if (!fill_refs_and_checksums_from_summary (summary, supported_ref_to_checksum, error))
     return FALSE;
 
-  /* Check that at least one of the refs has a non-%NULL checksum set, otherwise
-   * we can discard this peer. */
-  g_hash_table_iter_init (&iter, supported_ref_to_checksum);
-  while (g_hash_table_iter_next (&iter,
-                                 (gpointer *) &ref,
-                                 (gpointer *) &checksum))
+  removed_refs = g_hash_table_foreach_remove (supported_ref_to_checksum, remove_null_checksum_refs_cb, NULL);
+  if (removed_refs > 0)
+    g_debug ("Removed %d refs from the list resolved from ‘%s’ (possibly bloom filter false positives)",
+             removed_refs, remote->name);
+
+  /* If none of the refs had a non-%NULL checksum set, we can discard this peer. */
+  if (g_hash_table_size (supported_ref_to_checksum) == 0)
     {
-      if (checksum != NULL)
-        return TRUE;
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                   "No matching refs were found in the summary file");
+      return FALSE;
     }
 
-  g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-               "No matching refs were found in the summary file");
-  return FALSE;
+  return TRUE;
 }
 
 /* Download the summary file from @remote, and return the bytes of the file in
@@ -661,7 +670,7 @@ get_checksums (OstreeRepoFinderAvahi  *finder,
       return FALSE;
     }
 
-  return get_refs_and_checksums_from_summary (summary_bytes, supported_ref_to_checksum, error);
+  return get_refs_and_checksums_from_summary (summary_bytes, supported_ref_to_checksum, remote, error);
 }
 
 /* Build some #OstreeRepoFinderResults out of the given #OstreeAvahiService by